inspector,http: support builtin http request bodies#62915
inspector,http: support builtin http request bodies#62915GrinZero wants to merge 18 commits intonodejs:mainfrom
Conversation
|
Review requested:
|
metcoder95
left a comment
There was a problem hiding this comment.
lgtm, it just seems that is gonna need a documentation for the new dcs
|
Thanks for the feedback. I’ve updated the documentation in Specifically, I added/updated entries for:
I also aligned the wording and structure with the existing Node.js docs style for built-in channels. |
|
A slight push, hoping that a maintainer will notice. |
|
Sorry for the ping, @legendecas If you have some time, could you please help review this PR and trigger CI? This PR fixes an issue where the network inspector does not show Thanks a lot! |
|
push, request-ci plz |
|
Ill try to review the PR later, I think it's important we make sure it doesn't leak memory or keep the body buffer reference alive, which can exhaust resource quickly @ShogunPanda ptal |
I'll go explore the issue you mentioned |
|
Thanks for raising this concern. I did a focused memory/lifetime check for this PR, mainly around two questions:
Based on the current implementation and local testing, I do not see evidence that this PR keeps the original JS buffers alive. For both request and response bodies, the payload bytes are copied into inspector-owned storage before being retained by So the inspector is retaining copied payload bytes, not the original userland Buffer objects. I also ran a repeated-request memory test with explicit |
|
The test plan above was proposed by AI, and I reviewed it. For the first test, it uses The second test is a stress test. It lowers |
ab906d5 to
194a5fd
Compare
|
I merged the main branch code to avoid CI errors. |
194a5fd to
ba8f093
Compare
Thanks 🙏 I think we should add the request-ci tag to trigger the next steps. I have synced the code of the main branch |
aace601 to
16da9f4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #62915 +/- ##
==========================================
+ Coverage 90.03% 90.05% +0.01%
==========================================
Files 713 713
Lines 224950 225186 +236
Branches 42532 42590 +58
==========================================
+ Hits 202542 202790 +248
+ Misses 14175 14174 -1
+ Partials 8233 8222 -11
🚀 New features to boost your workflow:
|
c0db72d to
7edffdf
Compare
|
What are the commits after Root cause
hasPostData: !request.writableEndedAt that moment, The fixDefer the
A Why the C++ side has to changeThe previous .setStack(v8_inspector_->captureStackTrace(true)->buildInspectorObject(0))That relied on an implicit invariant: when JS calls Once the emission is deferred, the trigger point becomes a The only correct fix is to capture the stack on the JS side at request-creation time via That in turn requires the C++ backend to accept a JS-supplied initiator object, which is why:
How the follow-up commits fit in
|
Gently push forward the progress |
7edffdf to
a24b943
Compare
|
PUSH |
Signed-off-by: GrinZero <774933704@qq.com>
a24b943 to
d0f603a
Compare
|
Push the review slightly, the next plan is to promote websocket native, websocket from http, and SSE native, this part of the bug fix will be more important than the basics, if it can be completed first, the other PRs are not prone to conflicts |

Summary
This PR adds builtin
http/httpsrequest-body support to network inspection, soNetwork.getRequestPostDatacan return text request bodies while preserving the existing rejection behavior for binary request bodies.It also moves builtin
httpresponse-body tracking to a raw-byte hook beforeIncomingMessagedecoding, so response inspection remains correct even when user code callsresponse.setEncoding(...).In addition, this PR extends
requestWillBeSentto accept initiator data from JS diagnostics payloads and validates structuredinitiator.stackobjects against the inspector protocol schema before forwarding them to DevTools.Problem
Builtin
http/httpsnetwork inspection currently emits:Network.requestWillBeSentNetwork.responseReceivedNetwork.loadingFinishedHowever, the builtin
httpclient path does not currently expose request-body data to the inspector. As a result,Network.getRequestPostDatacannot return POST data for builtinhttp/httpsrequests.There are two related gaps as well:
IncomingMessage'data'events are not a stable source of raw bytes. If user code callsresponse.setEncoding('utf8'), the chunks observed by userland become strings, while the inspector protocol expects byte-oriented payloads.requestWillBeSentneeds to accept JS-provided initiator data, but structured stack trace input must be validated before it is forwarded as protocol data.Approach
1. Reuse the existing request buffering pipeline
This change does not alter the CDP schema or the existing buffering behavior in
NetworkAgent.Instead, it reuses the current pipeline already used by other transports:
2. Add builtin
httprequest-body diagnostics eventsThe builtin
httpclient now publishes:http.client.request.bodyChunkSenthttp.client.request.bodySentThese events are emitted from the
ClientRequestwrite path before HTTP framing is applied, so the inspector sees the original user payload rather than chunked-transfer framing bytes.3. Capture builtin
httpresponse bytes before decodingFor responses, this PR avoids relying on
IncomingMessage.on('data')innetwork_http.js.Instead, it adds:
http.client.response.bodyChunkReceivedThis hook runs before
Readable.setEncoding()transforms chunks for userland, so the inspector always receives raw bytes.4. Support and validate network initiator payloads
Network.requestWillBeSentcan now use JS diagnostics payloads to provide initiator data.On the C++ side, the incoming JS object is converted into inspector protocol values, and
initiator.stackis validated against theRuntime.StackTraceschema before being attached to the event. If the initiator payload is malformed, the event is rejected with a clear error instead of forwarding bad protocol data to the frontend.If JS does not provide an initiator, the existing behavior remains unchanged: the inspector captures the current stack trace and emits a default
scriptinitiator.5. Document the new diagnostics channels
This PR also updates
diagnostics_channeldocs to cover the new builtin HTTP client request/response body channels and their message shapes.Behavior
After this change:
httpandhttpsPOST requests with UTF-8 text bodies are available throughNetwork.getRequestPostDatahttpresponse inspection continues to work even if user code callsresponse.setEncoding('utf8')Network.requestWillBeSentcan carry JS-provided initiator stack dataTests
This PR adds and extends coverage in:
test/parallel/test-diagnostics-channel-http.jstest/parallel/test-inspector-network-http.jstest/parallel/test-inspector-network-arbitrary-data.jstest/parallel/test-inspector-emit-protocol-event-errors.jsThe updated tests cover:
write()andend()httpandhttpsNetwork.getRequestPostDataresponse.setEncoding('utf8')Verification
Verified locally with:
Refs